Units::teleport: keep occupancy flags while other units remain on the tile - #5946
Alistair-Afton wants to merge 2 commits into
Conversation
| if occ then | ||
| occ.unit = false | ||
| occ.unit_grounded = false | ||
| end | ||
| end | ||
| for _, unit in ipairs(units) do | ||
| local occ = tile_occupancy(unit.pos) | ||
| if occ then | ||
| if unit.flags1.on_ground then | ||
| occ.unit_grounded = true | ||
| else | ||
| occ.unit = true | ||
| end |
There was a problem hiding this comment.
I don't think this works. occ is not a pointer to the tile occupancy, it is a copy of it.
we don't have a setter yet, so you need to use this after line 47 and line 56:
dfhack.maps.getTileBlock(pos).occupancy[pos.x%16][pos.y%16] = occ
(untested code)
substitute unit.pos for the line 56 copy.
you can skip a nil test, the tile is known to exist, so the block also exists.
There was a problem hiding this comment.
I checked this in-game: getTileFlags pushes the pointer returned by Maps::getTileOccupancy, which is &block->occupancy[x&15][y&15], so the Lua object is a live ref into the block (same address as getTileBlock(pos).occupancy[x%16][y%16]), and direct field writes do propagate. The mutation sites work as written; the only change needed was reading via getTileFlags, which I have now adopted.
There was a problem hiding this comment.
Hmmm I'll experiment. verified it is a live pointer.
I have always treated it as a copy, so I've been doing extra work. (I know the C++ version is a pointer.)
static int maps_getTileFlags(lua_State *L)
{
auto pos = CheckCoordXYZ(L, 1, true);
Lua::PushDFObject(L, Maps::getTileDesignation(pos));
Lua::PushDFObject(L, Maps::getTileOccupancy(pos));
return 2;
}so that doesn't copy, it is live data, interesting.
in contrast, dfhack.maps.getTileType() returns an integer, not a pointer to a uint16_t. it doesn't have a setter either.
it's been on my TODO list to add setters for these, but clearly getTileFlags doesn't need a separate setter.
Caged units and in-flight projectiles do not set tile occupancy flags, so they must not keep flags alive when other units leave a tile. Test now uses dfhack.maps.getTileFlags, requires the destination tile to be walkable, and returns a failure reason from free_tile_near.
|
For context on the "needs attention from ab9rf or Quietust" point: ab9rf already verified the underlying behavior on #5924 — from reverse engineering, All of SilasD's special-case items are addressed in 86463ce plus the test-side fixes: caged and projectile units are skipped in the recompute scan (verified in-game that they set no occupancy), the source tile resolves through |
There was a problem hiding this comment.
I think this covers my issues. All good to go.
if some user is teleporting a ghost around, or teleporting a unit onto a ghost's tile, they have bigger problems than occupancy flags.
as an aside, there is a flying unit flag but it has nothing to do with block occupancy.
from memory, it's unit.enemy.caste_flags.FLIER, and clearing it will cause a flying unit to plummet to the ground. this group of flags are a copy of the master flags in the unit's creature_raw's caste_raw.flags, and they are not "sticky" -- they are recreated during the map load.
2 participants
Summary
Fixes #5938.
Units::teleportcleared the source tile'sunit_groundedflag unconditionally, so teleporting one grounded unit away from a tile left the flag unset even when another grounded unit still occupied it. The game itself only clears the flag when the last grounded unit leaves the tile.teleport now records which tiles still have a grounded or standing unit on them (honoring the 3x3 EQUIPMENT footprint) and only clears
unit_grounded/unitwhen the departing unit was the last of its kind there.Testing
test/modules/units_fortress.luacovers both directions: teleporting one of two grounded units off a tile keepsunit_grounded, teleporting one of two standing units keepsunit, and removing the last unit still clears the flag.